Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding projection control and map instance logic #132

Draft
wants to merge 7 commits into
base: RD-369
Choose a base branch
from

Conversation

jonathanlurie
Copy link
Collaborator

@jonathanlurie jonathanlurie commented Oct 30, 2024

This is adding the projection control as well as Map class methods to address the projection change in an easier and more complete way than Maplibre

This also include the readme documentation about projections

@jonathanlurie jonathanlurie marked this pull request as draft October 31, 2024 08:57
/**
* Returns whether a globe projection is currently being used
*/
isGlobeprojection(): boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capital "P" here

Comment on lines +355 to +369
## Field of view (FOV)
The internal camera has a default vertical field of view [*(wikipedia)*](https://en.wikipedia.org/wiki/Field_of_view) of a wide ~36.86 degrees. In globe mode, such a large *FOV* reduces the amount of the Earth that can be seen at once and exaggerates the central part, comparably to a fisheye lens. In many cases, a narrower *FOV* is preferable. Here is how to update if:

```ts
// Ajust de FOV, with values from 1 to 50
map.transform.setFov(10);
map.redraw();
```
> 📣 *__Note:__* with the Mercator projection, it is possible to set a FOV of `0`, which yields a true orthographic projection [*(wikipedia)*](https://en.wikipedia.org/wiki/Orthographic_projection), but the globe projection does not allow this.

Here is a table of FOV comparison:
| 01° | 10° | 20° | 30° | 40° | 50° |
| :--------: | :-------: |:-------: |:-------: |:-------: |:-------: |
| ![](images/screenshots/fov_1.jpeg) | ![](images/screenshots/fov_10.jpeg) | ![](images/screenshots/fov_20.jpeg) | ![](images/screenshots/fov_30.jpeg) | ![](images/screenshots/fov_40.jpeg) | ![](images/screenshots/fov_50.jpeg) |

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking whether this is not too specific and maybe does not need to be documented directly in the readme? (Especially since it adds several megabytes of data to the repo.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there is now a public method to adjust the FoV nicely: maplibre/maplibre-gl-js@d3f2bca

Comment on lines +379 to +380
> 📣 *__Note:__* Terrain is not fully compatible with the globe projection yet so it's better to disable it at low zoom level (from afar) and to choose the Mercator projection at higher zoom level (from up close).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is getting fixed soon: maplibre/maplibre-gl-js#4977

Copy link

@birkskyum birkskyum Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda, yes, but there's a few gotchas that'll persist even after my PR is in, that could justify keeping the recommendation as is.

There's still an issue with the poles on the globe if terrain is active, which often will be visible at low zoom levels. Disabling terrain from the globe will make it look better. - maplibre/maplibre-gl-js#4984.

There's not support for fog on the globe yet with terrain, even after the transition to mercator at high zoom levels - due to the way it's disabled, switching to mercator projection will allow the fog to render at high zoom. Tickets here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the advice @birkskyum !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants